Add support for encryption via pantalaimon #231
Conversation
| } | ||
|
|
||
| export interface PresenceEvent { | ||
| content: { |
There was a problem hiding this comment.
If the bridge shall be able to deal with home servers that are not spec-compliant, all properties would be marked optional and tested to have the right type (including the root object.
There was a problem hiding this comment.
If the bridge shall be able to deal with home servers that are not spec-compliant
We don't plan to do that at all. Writing a bridge would be a headache if that were the case :D. I've merely copied https://matrix.org/docs/spec/client_server/r0.6.0#m-presence which only has one required field.
src/components/intent.ts
Outdated
| encryption?: { | ||
| sessionPromise: Promise<ClientEncryptionSession|null>; | ||
| sessionCreatedCallback: (session: ClientEncryptionSession) => Promise<void>; | ||
| ensureSyncingCallback: () => Promise<void>; |
There was a problem hiding this comment.
Variable naming: Having looked at all occurrences, I'm not really sure what ensureSyncingCallback means.
There was a problem hiding this comment.
I've renamed it to ensureClientSyncingCallback. Essentially, it ensures the intent's client is /syncing before it performs any encrypted actions.
| ...result, | ||
| }; | ||
| if (this.encryption) { | ||
| this.encryption.sessionPromise = Promise.resolve(session); |
There was a problem hiding this comment.
I don't understand what's happening here.
The encryption object is coming from opts, so I guess the parent of this object will later check this promise and get the value???
Why is this a promise and not the value?
There was a problem hiding this comment.
Yes, this is a tricky one. In order to not break certain APIs, we have to ensure that the creation of an intent is not asynchronous. This means we can't do any store lookups, any API calls or anything until the intent is used. The problem with this is it means the first opportunity to do anything is in ensureRegistered, which is always called before we perform an intent action.
So things like the session which we need to pass to the client have to be passed as a promise to unwrap once we call ensureRegistered. Ideally we wouldn't be doing this at this stage, but it's that or breaking API compatibility, which felt like it would lead to a lot of work on the other bridges. On the whole passing a promise isn't the ultimate evil.
| const encryptionOpts = this.opts.bridgeEncryption; | ||
| if (encryptionOpts) { | ||
| clientIntentOpts.encryption = { | ||
| sessionPromise: encryptionOpts.store.getStoredSession(userId), |
There was a problem hiding this comment.
I guess here lies the answer why sessionPromise is a promise and not directly the session value..
because we can't await the promise here and we want to forward any errors to the parent.
There was a problem hiding this comment.
Ah yes, see above.
|
Note: We need matrix-js-sdk > 8.3.0 in order for this to work, but it hasn't been released yet. In order to make the CI pass I've included the latest, but we must be careful to only release a version of the bridge once the dependency has landed. |
Half-Shot
changed the title
|
Shall merge once matrix-js-sdk has its RC out. |
Half-Shot
force-pushed
the
hs/encryption-pan
branch
from
6c5e841 to
22bd557
Compare
This PR enables support for encryption for all bridges, designed to be a drop in upgrade and requiring a few options to be set.
Consequently there is a fair amount of callback fiddling and jumping around the stack to ensure that
getIntent()continues to not make any calls until the user requires it, and that the bridge logs users in so they can interact with pan.The meat of the changes lie in two places:
ensureRegisteredfunction modified to also set up encryption for the client. This involves causing it to fetch asessionfrom a datastore, or log in otherwise.EncryptedEventBroker. The broker tries to ensure that when an event arrives across the AS stream, the event is caught, and the correct event is fetched over sync and passed to onEvent. This involves some dances around retaining events temporarily in memory, and ensuring that only one event is ever handled.onEphemeralEventcan be used to handle these.This PR relies on a few dependencies, which are noted in issue #230.
This PR will likely be the last thing to be merged, so this PR fixes #230.